explorer: extract freshSelectionToken() primitive (closes #187 step 1) by rdhyee · Pull Request #188 · isamplesorg/isamplesorg.github.io

rdhyee · 2026-05-09T18:35:39Z

Summary

Step 1 of #187 (post-mortem on PR #186's 6-round Codex loop). Behavior-preserving refactor.

What changed

The same class of bug surfaced eight times across six review rounds: async work mutating selection state without checking that a newer user event had already superseded it. The duct-tape was a _selGen counter inlined at three call sites with slightly different shapes — each site bumped the counter, captured the snapshot, and rolled its own stale-check arrow.

This PR extracts that pattern as freshSelectionToken() (~10 lines) in the zoomWatcher cell:

function freshSelectionToken() {
    viewer._selGen = (viewer._selGen || 0) + 1;
    const gen = viewer._selGen;
    return () => gen !== viewer._selGen;
}

Each user-input event handler that touches selection now does:

const isStale = freshSelectionToken();
...
await someWork();
if (isStale()) return;

isStale is also passed through hydrateClusterUI's existing predicate parameter so nested awaits bail before touching the DOM.

Sites refactored

Site	Before	After
Source-filter handler	`viewer._selGen = (viewer._selGen \|\| 0) + 1; const filterSelGen = viewer._selGen;` + 4 `filterSelGen !== viewer._selGen` checks	`const isStale = freshSelectionToken();` + 4 `isStale()` checks
Hashchange handler	inline bump + `const selGen = viewer._selGen;` + 2 manual checks	`const isStale = freshSelectionToken();` + 2 `isStale()` checks
Boot deep-link	inline bump + `const bootSelGen = ...; const isBootStale = () => ...;` + 4 `isBootStale()` checks	`const isStale = freshSelectionToken();` + 4 `isStale()` checks

After the refactor, raw _selGen mentions exist only inside the helper. No other change in behavior.

Doc update

EXPLORER_STATE.md §4 adds:

A row for viewer._selGen (was previously undocumented, slipped in over the 6 rounds).
A new Async-selection invariant subsection that pins the rule: any async work that updates _globeState / URL hash / side-panel DOM must check isStale() after every await.

YAGNI

Step 2 of #187 (consolidating selection mutations into a selectSample / selectCluster / clearSelection controller) is deferred per the synthesis with Codex on that issue — only worth doing if the next selection feature again has to touch click + boot + hashchange + filter + URL. Three call sites isn't enough to force consolidation. A wholesale state-machine rewrite is not justified.

Verified locally

Re-ran the same three-step regression from PR #186's verification:

step	result (vs. pre-refactor)
Boot at `#h3=843f6d3ffffffff`	identical: cluster card populated, 30 samples
Uncheck SESAR (cluster's source OpenContext stays)	identical: cluster card unchanged, samples re-rendered, `&h3=` preserved
Uncheck OpenContext (cluster's own source)	identical: card cleared, samples cleared, `&h3=` dropped

Out of scope

Playwright stale-async regression test (deferred per explorer: extract _selGen freshness primitive (post-mortem on PR #186 6-round Codex loop) #187 synthesis — timing-flaky in headless; manual DevTools recipe is enough until something forces our hand).
Halo crescent / disappearing-dots over terrain (explorer: halo crescents / disappearing dots over hilly terrain (root cause not yet identified) #185 — separately tracked).

Refs #187 (post-mortem + Codex synthesis), #186 (origin PR), EXPLORER_STATE.md (state contract).

🤖 Generated with Claude Code

…#187 step 1) Six rounds of Codex review on PR isamplesorg#186 surfaced the same class of bug eight times: async work mutating selection state without checking that a newer user event had already superseded it. The duct-tape was a `_selGen` counter inlined at three call sites with slightly different shapes (filter handler, hashchange handler, boot deep-link). Each site bumped the counter, captured the snapshot, and rolled its own stale-check arrow. Extract the pattern as `freshSelectionToken()` in zoomWatcher (~10 lines): bumps `viewer._selGen`, returns an `isStale()` closure. The four call sites now just do: const isStale = freshSelectionToken(); ... await someWork(); if (isStale()) return; — with `isStale` passed into hydrateClusterUI's existing predicate parameter for nested-await freshness. Behavior-preserving refactor: verified locally that the three round-trips from PR isamplesorg#186's verification (boot at #h3=, uncheck non-cluster source, uncheck cluster's own source) produce identical results. Also documents the async-selection invariant explicitly in EXPLORER_STATE.md §4 (a new row for `viewer._selGen` plus an "Async-selection invariant" subsection) so future selection-touching code finds the rule before tripping the wire. Per isamplesorg#187 conversation between Claude Code and Codex: Step 2 of that issue (consolidating selection mutations into a `selectSample` / `selectCluster` / `clearSelection` controller) is YAGNI-deferred until the next feature actually has to touch click + boot + hashchange + filter + URL paths. A wholesale state-machine rewrite is not justified. Refs isamplesorg#187, isamplesorg#186. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

rdhyee

Finding:

P2: freshSelectionToken() does not cover the click path that the new invariant says it covers. EXPLORER_STATE.md says cluster/sample click handlers call freshSelectionToken(), but the helper is defined inside the later zoomWatcher cell, while the Cesium click handler is in the earlier viewer cell. The click handler still has async awaits with no freshness check: sample detail load and cluster nearby-samples load. A slow cluster click can still call updateSamples(samples) after the user has clicked a different sample/cluster. Either move the primitive to shared helper scope and use it in the viewer click handler, or narrow the docs/claim and leave click as explicit follow-up.

Verification: quarto render explorer.qmd and git diff --check origin/main...HEAD both passed.

Codex's review of PR isamplesorg#188 caught a P2 inconsistency: EXPLORER_STATE.md's async-selection invariant claimed coverage of cluster/sample click handlers, but freshSelectionToken() was defined inside the zoomWatcher cell — the click handler lives in the earlier viewer cell and couldn't see it. The click handler still had two unguarded awaits (sample detail load, cluster nearby-samples load) where a slow earlier click could repaint the side panel after the user clicked a different sample/cluster. Fix: move `freshSelectionToken(v)` to top-level OJS scope alongside `readHash` / `buildHash` so both cells can reach it. Take the viewer as a parameter (rather than closing over it) since the function is now defined before the viewer cell runs. Apply `isStale()` checks at the click handler's two await sites: sample-detail post-await and pre-DOM-write, cluster-nearby same. Doc adjusted to reflect the top-level definition. Behavior-preserving regression check: identical results to PR isamplesorg#186's verification suite (boot at #h3=, unchecked-non-cluster source, unchecked cluster source). Refs isamplesorg#187 (post-mortem), isamplesorg#188 (origin refactor PR — Codex's review of that diff surfaced this). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

rdhyee · 2026-05-09T19:22:49Z

Pushed `797605a` addressing Codex's P2 finding:

The original PR put `freshSelectionToken()` in the zoomWatcher cell, but the cluster/sample click handler lives in the earlier viewer cell and couldn't see it — yet `EXPLORER_STATE.md`'s invariant claimed coverage of clicks. Two unguarded awaits in the click handler (sample detail load, cluster nearby-samples load) were the actual race surfaces: a slow earlier click could repaint the side panel after the user clicked a different sample/cluster.

Fix:

Move `freshSelectionToken(v)` to top-level OJS scope alongside `readHash`/`buildHash`. Takes `viewer` as a parameter (defined before the viewer cell runs).
Apply `isStale()` to the click handler's two await sites — sample-detail and cluster-nearby. Same shape as the other four sites.
Updated `EXPLORER_STATE.md` invariant text to reflect the top-level definition.

Behavior-preserving regression check passes (PR #186's three-step suite returns identical results).

This makes the fifth call site of `freshSelectionToken` and the seventh `isStale()` check. The doc claim now matches the code.

rdhyee

Finding:

P3: EXPLORER_STATE.md has one stale location note for _selGen. The row for viewer._selGen still says freshSelectionToken() is “in zoomWatcher cell,” but the latest patch correctly moved it top-level so both the viewer click handler and zoomWatcher can use it. The invariant text below is already updated; this row just needs the same wording.

No runtime issues found in the re-review. The click path now uses freshSelectionToken(v) and guards both async repaint sites.

Verification: quarto render explorer.qmd and git diff --check origin/main...HEAD both passed.

Codex's P3 nit: the table row still said "in zoomWatcher cell" while the helper is now top-level. The invariant text below was already correct; this just aligns the row.

rdhyee commented May 9, 2026

View reviewed changes

docs: sync _selGen row in EXPLORER_STATE.md to top-level helper location

56d0064

Codex's P3 nit: the table row still said "in zoomWatcher cell" while the helper is now top-level. The invariant text below was already correct; this just aligns the row.

rdhyee merged commit 218e91f into isamplesorg:main May 9, 2026
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

explorer: extract freshSelectionToken() primitive (closes #187 step 1)#188

explorer: extract freshSelectionToken() primitive (closes #187 step 1)#188
rdhyee merged 3 commits intoisamplesorg:mainfrom
rdhyee:explorer-with-freshness

rdhyee commented May 9, 2026

Uh oh!

rdhyee left a comment

Uh oh!

rdhyee commented May 9, 2026

Uh oh!

rdhyee left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

rdhyee commented May 9, 2026

Summary

What changed

Sites refactored

Doc update

YAGNI

Verified locally

Out of scope

Uh oh!

rdhyee left a comment

Choose a reason for hiding this comment

Uh oh!

rdhyee commented May 9, 2026

Uh oh!

rdhyee left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant